Skip to content

Add error handing in OrderService._place_limit_order()#119

Open
djflux wants to merge 6 commits intorhettre:mainfrom
djflux:1-place_limit_order-error-handling
Open

Add error handing in OrderService._place_limit_order()#119
djflux wants to merge 6 commits intorhettre:mainfrom
djflux:1-place_limit_order-error-handling

Conversation

@djflux
Copy link

@djflux djflux commented Jan 2, 2025

As of main commit 72d9e38 the code for the _place_limit_order() method in the OrderService class in order_service.py throws an exception if limit_order_gtc_[buy|sell] errors out.

This pull request adds checking in _place_limit_order() that is similar to the checking located in OrderClass.fiat_market_sell().

This code has been tested with both a fiat_limit_buy and a fiat_limit_sell for an INSUFFICIENT_FUND case, and provides a unit test for the new code.

@rhettre
Copy link
Owner

rhettre commented Jan 4, 2025

Thanks for catching this inconsistency between _place_limit_order() and fiat_market_sell()

I'm wondering if it makes more sense to follow this solution:

  1. Instead of duplicating the error handling logic, let's move the common error checking into _log_order_result
  2. Have _log_order_result raise the exception when success is False
  3. This way we maintain consistent error handling across all order types in one place

Let me know what you think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants